Skip to content

Replace gRPC download call in proxy with local download_task using task manager#1915

Merged
gaius-qi merged 10 commits into
mainfrom
copilot/implement-download-task-in-proxy
Jul 14, 2026
Merged

Replace gRPC download call in proxy with local download_task using task manager#1915
gaius-qi merged 10 commits into
mainfrom
copilot/implement-download-task-in-proxy

Conversation

Copilot AI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

The proxy downloaded tasks through the dfdaemon download gRPC client over a unix socket (dfdaemon_download_client.download_task in proxy/mod.rs), adding unnecessary gRPC transport overhead to every proxied download. This implements the same download_task logic locally in the proxy using resource::task::Task functions directly, removing the gRPC hop.

Local download_task in proxy/mod.rs

Mirrors the gRPC download_task handler in grpc/dfdaemon_download.rs, but calls the task manager directly:

  • Generates task/host/peer IDs via task.id_generator (content, blob-digest-based, and URL-based TaskIDParameter).
  • Calls task.download_started, resolves the range from the request header via get_range, then spawns task.download writing into a local mpsc channel (DOWNLOAD_STREAM_BUFFER_SIZE), returning the receiver as the response stream.
  • Preserves the success path (download_finished, output-path hard-link/copy checks, digest verification), failure path (download_failed, backend error details), and all download task metrics.
  • Drops gRPC-only concerns: tonic Request/Response wrappers, OpenTelemetry context extraction, and blocklist checks.

Local prefetch_task

Mirrors grpc::prefetch_task but invokes the local download_task instead of creating a gRPC client. download_task returns a boxed future to break the recursive future type between download_task and prefetch_task.

proxy_via_dfdaemon rework

// Before: gRPC stream over unix socket
let response = dfdaemon_download_client.download_task(request).await?;
let mut out_stream = response.into_inner();

// After: local mpsc stream from the task manager
let mut out_stream = download_task(config.clone(), task.clone(), request).await?;
  • Consumes mpsc::Receiver<Result<DownloadTaskResponse, Status>> instead of a tonic stream.
  • Handles ClientError::BackendError directly (status code + headers) instead of decoding tonic Status details JSON.
  • Removes ResourceExhausted/PermissionDenied mappings that only originated from gRPC server middleware.

Cleanup

Removed DfdaemonDownloadClient construction from Proxy::run and its parameter from all proxy handlers (handler, http_handler, https_handler, upgraded_tunnel, upgraded_handler, registry mirror handlers).

…task.rs

- Move `download_task` and `prefetch_task` functions into new
  `proxy/task.rs`
- Rename them to `task::download` and `task::prefetch` with public
  visibility
- Clean up unused imports in `proxy/mod.rs` after the extraction
- Declare `task` as a public submodule in `proxy/mod.rs

Signed-off-by: Gaius <gaius.qi@gmail.com>
@gaius-qi gaius-qi added the enhancement New feature or request label Jul 10, 2026
@gaius-qi gaius-qi added this to the v2.6.0 milestone Jul 10, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 180 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.84%. Comparing base (211ad93) to head (c8112af).

Files with missing lines Patch % Lines
dragonfly-client/src/proxy/task.rs 0.00% 138 Missing ⚠️
dragonfly-client/src/proxy/mod.rs 0.00% 34 Missing ⚠️
dragonfly-client/src/dynconfig/block_list.rs 0.00% 3 Missing ⚠️
dragonfly-client/src/dynconfig/mod.rs 0.00% 3 Missing ⚠️
dragonfly-client/src/bin/dfdaemon/main.rs 0.00% 1 Missing ⚠️
dragonfly-client/src/grpc/dfdaemon_download.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1915      +/-   ##
==========================================
- Coverage   46.11%   45.84%   -0.28%     
==========================================
  Files          93       94       +1     
  Lines       23637    23791     +154     
==========================================
+ Hits        10901    10906       +5     
- Misses      12736    12885     +149     
Files with missing lines Coverage Δ
dragonfly-client-core/src/error/mod.rs 59.09% <ø> (ø)
dragonfly-client/src/grpc/mod.rs 0.00% <ø> (ø)
dragonfly-client/src/bin/dfdaemon/main.rs 4.98% <0.00%> (-0.02%) ⬇️
dragonfly-client/src/grpc/dfdaemon_download.rs 4.92% <0.00%> (ø)
dragonfly-client/src/dynconfig/block_list.rs 77.84% <0.00%> (ø)
dragonfly-client/src/dynconfig/mod.rs 0.00% <0.00%> (ø)
dragonfly-client/src/proxy/mod.rs 0.00% <0.00%> (ø)
dragonfly-client/src/proxy/task.rs 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gaius-qi added 3 commits July 13, 2026 22:22
- Move `block_list` module from `grpc` to `dynconfig` for better
  cohesion
- Embed `BlockList` instance directly into `Dynconfig`, sharing its
  `Arc<RwLock<Data>>`
- Replace standalone `BlockList` fields in handlers/proxy with
  `dynconfig` reference
- Add `PermissionDenied` error variant; use it instead of
  `Unauthorized` for block-list rejections

Signed-off-by: Gaius <gaius.qi@gmail.com>
…etch

- Replace `ClientError::Unauthorized` with
  `ClientError::PermissionDenied` for blocklist rejections
- Remove pre-cloning of request for prefetch; clone at download call
  instead
- Extract `download` from request after streaming starts, early-return
  on missing field
- Pass `download_task_request` directly to prefetch instead of a
  separate clone

Signed-off-by: Gaius <gaius.qi@gmail.com>
@gaius-qi gaius-qi marked this pull request as ready for review July 14, 2026 06:40
@gaius-qi gaius-qi requested review from a team as code owners July 14, 2026 06:40
@gaius-qi gaius-qi requested review from Zephyrcf and anjia0532 July 14, 2026 06:40
@github-actions github-actions Bot requested review from jim3ma and yyzai384 July 14, 2026 06:40
- Strip port 80 from `Host` in HTTP proxy, port 443 in HTTPS proxy
- Keeps non-default ports in `Host` for containerd compatibility
- Fixes signature mismatch for presigned URLs that sign `host`
  without the default port

Signed-off-by: Gaius <gaius.qi@gmail.com>
@gaius-qi gaius-qi enabled auto-merge (squash) July 14, 2026 08:41

@hyy0322 hyy0322 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gaius-qi gaius-qi merged commit 147a025 into main Jul 14, 2026
7 checks passed
@gaius-qi gaius-qi deleted the copilot/implement-download-task-in-proxy branch July 14, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants